Skip to content

feat(cli): Add --model flag and WARDEN_MODEL env var#16

Merged
dcramer merged 2 commits intomainfrom
feat/model-cli-flag
Jan 29, 2026
Merged

feat(cli): Add --model flag and WARDEN_MODEL env var#16
dcramer merged 2 commits intomainfrom
feat/model-cli-flag

Conversation

@dcramer
Copy link
Member

@dcramer dcramer commented Jan 29, 2026

  • Add --model / -m CLI flag to override the model used for analysis
  • Add WARDEN_MODEL environment variable support as a fallback
  • Clear precedence order: CLI flag > trigger config > defaults config > env var > SDK default

Allow users to override the model used for analysis via CLI flag or
environment variable, with clear precedence:
1. --model CLI flag (highest)
2. trigger.model (warden.toml)
3. defaults.model (warden.toml)
4. WARDEN_MODEL env var
5. SDK default (lowest)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@vercel
Copy link

vercel bot commented Jan 29, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
warden Ready Ready Preview, Comment Jan 29, 2026 10:16pm

Request Review

Copy link
Contributor

@sentry-warden sentry-warden bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-review

security-review: Found 5 issues (3 medium, 1 low, 1 info)

src/cli/main.ts Outdated
// Build skill tasks
const runnerOptions: SkillRunnerOptions = { apiKey, abortController };
// Model precedence: CLI flag > WARDEN_MODEL env var > SDK default
const model = options.model ?? process.env['WARDEN_MODEL'];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Unvalidated environment variable used for model selection

The code reads from process.env['WARDEN_MODEL'] without validation and passes it directly to the SDK. If this model parameter is used in API calls, command construction, or file operations, an attacker with control over environment variables could potentially inject malicious values. While the impact depends on how the SDK uses the 'model' parameter, environment variables should generally be validated before use, especially when they influence external API calls or system behavior.

Suggested fix: Validate the model parameter against an allowlist of known valid model names, or at minimum sanitize the input to ensure it matches expected patterns (e.g., alphanumeric with hyphens).

Suggested change
const model = options.model ?? process.env['WARDEN_MODEL'];
let model = options.model ?? process.env['WARDEN_MODEL'];
// Validate model parameter if provided
if (model && !/^[a-zA-Z0-9_\-\.]+$/.test(model)) {
reporter.error('Invalid model name format');
throw new Error('Model name contains invalid characters');
}

warden: security-review

Comment on lines 73 to 88
@@ -73,6 +85,6 @@ export function resolveTrigger(trigger: Trigger, config: WardenConfig): Resolved
maxFindings: trigger.output?.maxFindings ?? defaults?.output?.maxFindings,
labels: trigger.output?.labels ?? defaults?.output?.labels,
},
model: trigger.model ?? defaults?.model,
model: cliModel ?? trigger.model ?? defaults?.model ?? envModel,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Unvalidated environment variable used in model configuration

The code reads from process.env['WARDEN_MODEL'] and uses it directly without validation or sanitization. While this is used for model selection (likely a string identifier), there's no validation that the value is from an expected set of models. If this model identifier is later used in command execution, path construction, or direct interpolation into commands/queries, it could lead to injection vulnerabilities. Additionally, an attacker with environment variable control could potentially cause unexpected behavior by providing malformed or malicious model identifiers.

Suggested fix: Add validation for the environment variable value against a whitelist of allowed model identifiers, or at minimum, sanitize the input to ensure it contains only expected characters (e.g., alphanumeric and hyphens). Consider using a validation schema similar to how WardenConfigSchema validates other configuration.

Suggested change
let envModel = process.env['WARDEN_MODEL'];
// Validate environment variable contains only safe characters
if (envModel && !/^[a-zA-Z0-9._-]+$/.test(envModel)) {
throw new ConfigLoadError('Invalid WARDEN_MODEL environment variable: contains unsafe characters');
}

warden: security-review

labels: trigger.output?.labels ?? defaults?.output?.labels,
},
model: trigger.model ?? defaults?.model,
model: cliModel ?? trigger.model ?? defaults?.model ?? envModel,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Potential Environment Variable Injection via WARDEN_MODEL

The code reads the WARDEN_MODEL environment variable and uses it directly to set the model configuration without validation or sanitization. If an attacker can control environment variables (e.g., in CI/CD pipelines, containerized environments, or shared hosting), they could potentially inject malicious model names or cause unexpected behavior. The precedence order places cliModel highest, but envModel is used as a fallback, which could override default configurations unexpectedly.

Suggested fix: Add validation for the WARDEN_MODEL environment variable to ensure it matches expected model names. Consider maintaining an allowlist of valid model identifiers and rejecting or warning on unexpected values.

Suggested change
model: cliModel ?? trigger.model ?? defaults?.model ?? envModel,
let envModel = process.env['WARDEN_MODEL'];
// Validate environment variable input
const ALLOWED_MODELS = ['claude-3-opus', 'claude-3-sonnet', 'claude-3-haiku', 'gpt-4', 'gpt-3.5-turbo'];
if (envModel && !ALLOWED_MODELS.includes(envModel)) {
console.warn(`Warning: Unknown model '${envModel}' specified in WARDEN_MODEL. Ignoring.`);
envModel = undefined;
}

warden: security-review

Model precedence is now: trigger.model > defaults.model > --model flag > WARDEN_MODEL env var

This ensures consistent behavior between config mode and file/git-ref modes,
where config settings take priority as the project-level defaults.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Copy link
Contributor

@sentry-warden sentry-warden bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-review

security-review: Found 4 issues (3 medium, 1 low)

Comment on lines +122 to +123
const model = defaultsModel ?? options.model ?? process.env['WARDEN_MODEL'];
const runnerOptions: SkillRunnerOptions = { apiKey, model, abortController };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Environment Variable Injection in Model Selection

The code reads from process.env['WARDEN_MODEL'] without validation and passes it directly to the model configuration. If an attacker can control environment variables (e.g., in a CI/CD pipeline or shared hosting environment), they could potentially inject malicious model names or attempt to manipulate the behavior of the application. While the actual exploitability depends on how the 'model' parameter is validated downstream, it's a best practice to validate or sanitize environment variable inputs, especially when they control application behavior.

Suggested fix: Add validation to ensure the model name matches expected patterns or is from an allowlist of valid model names before using it.

Suggested change
const model = defaultsModel ?? options.model ?? process.env['WARDEN_MODEL'];
const runnerOptions: SkillRunnerOptions = { apiKey, model, abortController };
const envModel = process.env['WARDEN_MODEL'];
// Validate model name if from environment
if (envModel && !/^[a-z0-9-]+$/.test(envModel)) {
reporter.error('Invalid model name in WARDEN_MODEL environment variable');
return 1;
}
const model = defaultsModel ?? options.model ?? envModel;

warden: security-review

config: WardenConfig,
cliModel?: string
): ResolvedTrigger {
const defaults = config.defaults;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Environment variable used directly without validation

The code reads process.env['WARDEN_MODEL'] and uses it directly in the model precedence chain without validation. If an attacker can control environment variables (e.g., in CI/CD pipelines, containerized environments, or through process spawning), they could potentially inject malicious model names or values that might be interpreted by downstream code in unintended ways. While the impact depends on how the model value is used later, accepting arbitrary strings from environment variables without validation is a security risk.

Suggested fix: Add validation to ensure the environment variable value matches expected patterns (e.g., allowed model names, format validation). Consider using a whitelist of allowed model names or at minimum, validate that it matches expected patterns.

Suggested change
const defaults = config.defaults;
const envModelRaw = process.env['WARDEN_MODEL'];
// Validate env var: alphanumeric, hyphens, dots, max 100 chars
const envModel = envModelRaw && /^[a-zA-Z0-9._-]{1,100}$/.test(envModelRaw)
? envModelRaw : undefined;

warden: security-review

labels: trigger.output?.labels ?? defaults?.output?.labels,
},
model: trigger.model ?? defaults?.model,
model: trigger.model ?? defaults?.model ?? cliModel ?? envModel,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Unvalidated environment variable used in model configuration

The WARDEN_MODEL environment variable is read from process.env and used directly without validation or sanitization. If this model value is later passed to an external API, command execution, or used in security-sensitive contexts, an attacker with control over environment variables could potentially inject malicious values. The impact depends on how the model value is used downstream - if it's passed to shell commands, file paths, or external services without validation, this could lead to injection vulnerabilities.

Suggested fix: Validate the environment variable against an allowlist of acceptable model values before using it. If the model names are known, use a whitelist approach. Otherwise, validate the format and reject suspicious characters.

Suggested change
model: trigger.model ?? defaults?.model ?? cliModel ?? envModel,
let envModel = process.env['WARDEN_MODEL'];
// Validate model name to prevent injection
if (envModel && !/^[a-zA-Z0-9._-]+$/.test(envModel)) {
console.warn(`Invalid WARDEN_MODEL format: ${envModel}. Ignoring.`);
envModel = undefined;
}

warden: security-review

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

// Build skill tasks
const runnerOptions: SkillRunnerOptions = { apiKey, abortController };
// Model precedence: defaults.model > CLI flag > WARDEN_MODEL env var > SDK default
const model = defaultsModel ?? options.model ?? process.env['WARDEN_MODEL'];
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Model precedence order is inverted from stated intent

High Severity

The model precedence implementation contradicts the PR description. The PR states "CLI flag > trigger config > defaults config > env var" but the code implements the opposite: defaultsModel ?? options.model ?? process.env['WARDEN_MODEL'] gives config higher priority than the CLI flag. When a user passes --model claude-opus-4-5-20250514, they expect it to override config settings, but currently config values win.

Additional Locations (1)

Fix in Cursor Fix in Web

@dcramer dcramer merged commit e38256d into main Jan 29, 2026
13 checks passed
@dcramer dcramer deleted the feat/model-cli-flag branch January 29, 2026 22:23
dcramer added a commit that referenced this pull request Feb 17, 2026
…raction (#156)

When chunks fail to analyze or finding extractions fail, users now see
detailed per-failure information at verbose levels instead of only
aggregate counts in the summary. Add three new events to the reporter spec:
hunk_failed (event #16), extraction_failure (event #17), and retry (event #18).

At Verbose (-v), per-hunk failure details appear inline during skill
execution. At Debug (-vv), extraction failures also show the first 200 chars
of the raw output that failed to parse. The summary now includes a "-v for
failure details" hint when failures are present and verbosity < Verbose.

Wire three existing but unused callbacks through the CLI layer:
- onHunkFailed: fired when SDK analysis fails
- onExtractionFailure: fired when JSON/LLM extraction fails
- onRetry: fired when retry attempts are made

Include failedHunks and failedExtractions in JSONL output for structured
tracking of analysis degradation. Add comprehensive tests covering all
three callback implementations in both TTY and Plain modes.

Update specs/reporters.md with complete documentation of the three new
events, the updated summary section with -v hint, JSONL record changes,
and master checklist updates. Add spec reference comments in source files
so developers know where to find the reporter specification.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
dcramer added a commit that referenced this pull request Feb 17, 2026
…raction (#156)

When chunks fail to analyze or finding extractions fail, users now see
detailed per-failure information at verbose levels instead of only
aggregate counts in the summary. Add three new events to the reporter spec:
hunk_failed (event #16), extraction_failure (event #17), and retry (event #18).

At Verbose (-v), per-hunk failure details appear inline during skill
execution. At Debug (-vv), extraction failures also show the first 200 chars
of the raw output that failed to parse. The summary now includes a "-v for
failure details" hint when failures are present and verbosity < Verbose.

Wire three existing but unused callbacks through the CLI layer:
- onHunkFailed: fired when SDK analysis fails
- onExtractionFailure: fired when JSON/LLM extraction fails
- onRetry: fired when retry attempts are made

Include failedHunks and failedExtractions in JSONL output for structured
tracking of analysis degradation. Add comprehensive tests covering all
three callback implementations in both TTY and Plain modes.

Update specs/reporters.md with complete documentation of the three new
events, the updated summary section with -v hint, JSONL record changes,
and master checklist updates. Add spec reference comments in source files
so developers know where to find the reporter specification.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
dcramer added a commit that referenced this pull request Feb 18, 2026
…raction (#162)

Surface per-failure details when chunks fail to analyze or finding
extractions fail, enabling users to debug failures without Sentry
access.

Previously, when a chunk failed or an extraction couldn't parse, users
only saw aggregate counts ("1 chunk failed to analyze") in the summary.
This made debugging impossible—they couldn't see which file or line
range failed, what error occurred, or what the raw output was. Operators
had to grep Sentry logs or ask engineers for help.

This PR adds three new events to the reporter spec (events #16–18) that
surface per-failure information at verbose levels:
- **hunk_failed** (event #16): When SDK analysis fails (SDK error, API
error, abort). Shows file:lineRange and error message.
- **extraction_failure** (event #17): When both regex and LLM fallback
fail to extract JSON. At Debug level, also logs the first 200 chars of
the output that failed to parse.
- **retry** (event #18): When a retry attempt is made. Shows attempt
number, max retries, delay, and error.

All three are gated on `Verbosity.Verbose` (enabled with `-v`), keeping
normal output clean. Debug level (`-vv`) adds output previews for
extraction failures.

The "-v for failure details" hint now appears in summaries when failures
are present but verbosity is below Verbose, guiding users to re-run with
verbose output.

Additional improvements:
- JSONL now includes `failedHunks` and `failedExtractions` counts (spec
gap is now closed)
- Spec updated with examples for all three reporters (TTY, Plain, Ink)
and all four verbosity levels
- Comprehensive tests covering both TTY and Plain modes,
Verbose/Debug/Normal verbosity levels

Fixes #156

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant